Skip to content

feature: replace container-interop/container-interop#96

Merged
weierophinney merged 5 commits into
laminas:3.11.xfrom
boesing:feature/remove-container-interop
Apr 4, 2022
Merged

feature: replace container-interop/container-interop#96
weierophinney merged 5 commits into
laminas:3.11.xfrom
boesing:feature/remove-container-interop

Conversation

@boesing

@boesing boesing commented Jul 27, 2021

Copy link
Copy Markdown
Member

PoC

@boesing boesing force-pushed the feature/remove-container-interop branch from aa302d4 to 306df04 Compare July 27, 2021 22:29
boesing
boesing commented Jul 27, 2021
View reviewed changes
Comment thread composer.json
boesing
boesing commented Jul 27, 2021
View reviewed changes
Comment thread src/autoload.php Outdated
Ocramius
Ocramius requested changes Jul 28, 2021
View reviewed changes
Comment thread composer.json
Comment thread src/AbstractPluginManager.php
Comment thread src/Exception/ExceptionInterface.php
Comment thread src/Exception/ServiceNotCreatedException.php
Comment thread src/Exception/ServiceNotFoundException.php
Comment thread src/autoload.php
@boesing boesing force-pushed the feature/remove-container-interop branch from 1758c7e to a4e53e5 Compare July 28, 2021 07:15
@boesing boesing mentioned this pull request Jul 28, 2021
Open
@boesing

boesing commented Jul 28, 2021

Copy link
Copy Markdown
Member Author

I've also added a request to psr/container regarding this topic.
php-fig/container#37

@boesing boesing requested a review from Ocramius July 28, 2021 10:04
weierophinney
weierophinney requested changes Jul 30, 2021
View reviewed changes

@weierophinney weierophinney left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a CS rule in our standard that is lowercasing class/namespace identifiers it does not recognized. I've pointed out one set, but there's more.

Comment thread test/AbstractFactory/ReflectionBasedAbstractFactoryTest.php Outdated
Comment thread test/AbstractFactory/ReflectionBasedAbstractFactoryTest.php Outdated
Comment thread test/AbstractFactory/ReflectionBasedAbstractFactoryTest.php Outdated
Comment thread test/AbstractFactory/ReflectionBasedAbstractFactoryTest.php Outdated
@weierophinney

weierophinney commented Aug 6, 2021

Copy link
Copy Markdown
Member

@boesing If php-fig/container#38 gets accepted and released, how does that change this PR? I'm assuming we should wait to see what happens there?

@boesing

boesing commented Aug 6, 2021 •
edited
Loading

Copy link
Copy Markdown
Member Author

@weierophinney exactly.
If psr/container replaces interop by itself, it does not need to be done here.
Would remove the replace stuff along with the autoload for class_alias, applying the changes requested in this PR and we're done.
I would prefer not to replace interop as we do not own it but if it gets rejected, I think we can do it (even if we shouldn't).

@boesing boesing mentioned this pull request Aug 6, 2021
Closed
@weierophinney

weierophinney commented Aug 17, 2021

Copy link
Copy Markdown
Member

Update for those following: see container-interop/container-interop#97 for details on the planned approach. In sum:

  • That package will do a v2.0.0 release that defines its own interfaces as class aliases of the PSR-11 interfaces.
  • We will adopt that in the next minor of this package, and, simultaneously, remove any references to container-interop internally. This will be a BC compatible, transparent change.
  • We will document that users can now refer directly to PSR-11 interfaces in their own code once they have set the new minor version as their minimum in their own application and/or library. We will also send out messages on social media and our blog to further reach for this initiative.
  • From there, we will decide whether to remove container-interop in the next minor or next major; most likely, it will be next major, as it will be a BC break for anybody upgrading from older minor versions.

This was referenced Nov 10, 2021
Closed
Closed
@Ocramius

Ocramius commented Feb 15, 2022

Copy link
Copy Markdown
Member

We kinda need to move on with this, to enable psr/container:^2 compatibility.

@weierophinney I see no activity on container-interop/container-interop - do you have any leverage there?

@Ocramius Ocramius removed their request for review February 15, 2022 07:39
@Ocramius Ocramius mentioned this pull request Feb 23, 2022
Closed
@boesing

This comment was marked as off-topic.

Sign in to view
@Ocramius

This comment was marked as off-topic.

Sign in to view
@boesing

This comment was marked as off-topic.

Sign in to view
@boesing

This comment was marked as off-topic.

Sign in to view
@boesing

This comment was marked as off-topic.

Sign in to view
@Ocramius Ocramius mentioned this pull request Mar 21, 2022
Closed
@boesing boesing changed the base branch from 3.8.x to 3.11.x April 4, 2022 09:11
boesing added 3 commits April 4, 2022 11:28
@boesing
feature: replace container-interop/container-interop
0732d65 
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing
qa: add integration test for container interop
4756d91 
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing
qa: add container-interop integration test to verify that upstream …
465f122 
…can still use interop interfaces

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing boesing force-pushed the feature/remove-container-interop branch from a4e53e5 to b6fb58a Compare April 4, 2022 09:29
@boesing boesing requested a review from weierophinney April 4, 2022 09:34
@boesing boesing requested a review from Ocramius April 4, 2022 09:34
boesing added 2 commits April 4, 2022 11:45
@boesing
qa: code-cleanup
19bd36e 
- PSR does not implement `Throwable` in 1.x versions.
- Suppress `AbstractPluginManager#__construct` error as it is not fixable without BC
- `LazyServiceFactoryTest` used old `PHPUnit_Framework_MockObject_MockObject` from ancient phpunit version

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing
qa: apply coding standard after rebasing
9f5b839 
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing boesing force-pushed the feature/remove-container-interop branch from ad80182 to 9f5b839 Compare April 4, 2022 09:45
weierophinney
weierophinney approved these changes Apr 4, 2022
View reviewed changes

@weierophinney weierophinney left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Net removal of code, simplification internally, and immediate PSR-11 compatibility.

I am so thrilled by this! 😍

@weierophinney weierophinney added this to the 3.11.0 milestone Apr 4, 2022
@weierophinney weierophinney merged commit ecddf00 into laminas:3.11.x Apr 4, 2022
@boesing boesing deleted the feature/remove-container-interop branch April 4, 2022 20:17
@imonteiro imonteiro mentioned this pull request Mar 23, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@weierophinney weierophinney weierophinney approved these changes
@Ocramius Ocramius Awaiting requested review from Ocramius

Assignees

No one assigned

Labels

Enhancement

Projects

None yet

Milestone

3.11.0

Development

Successfully merging this pull request may close these issues.

4 participants

@boesing @weierophinney @Ocramius @chinmoym2004